fix: improve error handling and add routing#41
fix: improve error handling and add routing#41kunal-bunkar wants to merge 2 commits intoAOSSIE-Org:mainfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughAdds react-router-dom, wraps the app with BrowserRouter, defines Routes in App ("/" → Home UI, "*" → NotFoundPage), and introduces a new Changes
Sequence Diagram(s)sequenceDiagram
participant User as "Client (Browser)"
participant Router as "BrowserRouter"
participant App as "App"
participant Home as "HomeRoute"
participant NotFound as "NotFoundPage"
User->>Router: Navigate to URL
Router->>App: Provide routing context + current path
alt path == "/"
App->>Home: Render HomeRoute
Home-->>User: Send Home UI
else unknown path
App->>NotFound: Render NotFoundPage
NotFound-->>User: Send 404 + Link to "/"
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/App.tsx`:
- Around line 9-10: The component currently hardcodes user-visible strings
(e.g., "Hello, OrgExplorer!", "404", "Page not found.", "Go home") inside the
App component and route JSX; extract these literals into the i18n/resource
system and replace the inline text with lookups (e.g., use t('home.title') or a
resources object) within App and any route components (refer to App / the Route
rendering that shows "404"/"Page not found."/"Go home"). Ensure keys are added
to the resource files and used via the existing i18n/getString utility or a new
simple wrapper so all visible strings in the component tree are read from
resources rather than hardcoded.
- Around line 16-24: The NotFoundRoute component is implemented inline in App as
function NotFoundRoute; extract it into a new file src/NotFoundPage.tsx as a
default-exported React component (e.g., export default function NotFoundPage())
that returns the same JSX (h1, p, Link from 'react-router-dom'), then update App
to import NotFoundPage and replace references to NotFoundRoute with the imported
NotFoundPage (and remove the original NotFoundRoute function from App).
In `@src/main.tsx`:
- Around line 9-11: The app currently uses BrowserRouter in main.tsx (wrapping
<App />) which requires the hosting to rewrite unknown paths to index.html for
direct navigations; verify the deployment configuration provides that SPA
fallback (rewrite/redirect unknown routes to index.html) and confirm in the PR,
or if the host cannot supply rewrites switch to HashRouter (imported from
react-router-dom and replace BrowserRouter with HashRouter around <App />) so
hard-refreshes/direct requests work without server rewrites.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 614d2e12-ba5d-47dd-97e1-4e847b467dfd
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
package.jsonsrc/App.tsxsrc/main.tsx
src/App.tsx
Outdated
| <h1>Hello, OrgExplorer!</h1> | ||
| </> |
There was a problem hiding this comment.
Externalize the routed page copy.
Hello, OrgExplorer!, 404, Page not found., and Go home are hardcoded in the component tree. Please move them into the project's resource files before expanding the route surface.
As per coding guidelines, "User-visible strings should be externalized to resource files (i18n)."
Also applies to: 19-21
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/App.tsx` around lines 9 - 10, The component currently hardcodes
user-visible strings (e.g., "Hello, OrgExplorer!", "404", "Page not found.", "Go
home") inside the App component and route JSX; extract these literals into the
i18n/resource system and replace the inline text with lookups (e.g., use
t('home.title') or a resources object) within App and any route components
(refer to App / the Route rendering that shows "404"/"Page not found."/"Go
home"). Ensure keys are added to the resource files and used via the existing
i18n/getString utility or a new simple wrapper so all visible strings in the
component tree are read from resources rather than hardcoded.
| <BrowserRouter> | ||
| <App /> | ||
| </BrowserRouter> |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Verify SPA fallback support for BrowserRouter.
This setup only handles misses after the client boots. A direct request or hard refresh on /xyz will still return a server 404 unless the deployed host rewrites unknown paths to index.html. Please confirm that rewrite exists, or use HashRouter if the target hosting cannot provide it.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main.tsx` around lines 9 - 11, The app currently uses BrowserRouter in
main.tsx (wrapping <App />) which requires the hosting to rewrite unknown paths
to index.html for direct navigations; verify the deployment configuration
provides that SPA fallback (rewrite/redirect unknown routes to index.html) and
confirm in the PR, or if the host cannot supply rewrites switch to HashRouter
(imported from react-router-dom and replace BrowserRouter with HashRouter around
<App />) so hard-refreshes/direct requests work without server rewrites.
Addressed Issues:
Fixes #40
Screenshots/Recordings:
Additional Notes:
This PR introduces minimal client-side routing using
react-router-dom:BrowserRouterinsrc/main.tsxsrc/App.tsxfor:/(home)*(404)src/NotFoundPage.tsxVerification:
npm run lintpassesChecklist
Summary by CodeRabbit